Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added more landing pads to the Airforce Command Center #735

Closed
wants to merge 1 commit into from

Conversation

Mailaender
Copy link
Member

Closes #87.

@sorcerer86pt
Copy link

sorcerer86pt commented Jun 24, 2020

Ok, tested this commit.
Some changes need to be done. As you can see in this video:

multiple airfields (see behavior after 30s mark)

When multiple airfields exists, the airplanes sometimes enter in loop trying to find a slot, when another one is open (mostly when building new planes, when coming back from bombing mission, they find the slot correctly)

An possible fix is at build time, find a open slot in all airfields that the player controls and then deploy that plane there.

Copy link

@sorcerer86pt sorcerer86pt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs some work when multiple airfields exists and a plane is built.

@pgwipeout
Copy link

Your video is not accessible unfortunately.

@sorcerer86pt
Copy link

sorcerer86pt commented Jun 24, 2020 via email

@sorcerer86pt
Copy link

sorcerer86pt commented Jun 24, 2020 via email

@Mailaender
Copy link
Member Author

An possible fix is at build time, find a open slot in all airfields that the player controls and then deploy that plane there.

Probably not possible with the way this hack works. Only 25% of the pads is constructing aircraft, the others are just for rearming.

@yaaaaa
Copy link

yaaaaa commented Jun 24, 2020

May be it needs to spawn/build the plane right on free slot, and without flying/jumping after build?

@sorcerer86pt
Copy link

sorcerer86pt commented Jun 24, 2020 via email

@MustaphaTR
Copy link
Member

Also from the video @sorcerer86pt sent looks like planes land facing 0, they should face 224. MustaphaTR/Romanovs-Vengeance@cc69deb

@Mailaender
Copy link
Member Author

Changed the landing facing and added production capabilities to the pads. I don't know why TakeOffOnCreation: false is ignored. We have a ticket for that #683.

OpenRA.Mods.RA2/Traits/SpawnNeighboringActors.cs Outdated Show resolved Hide resolved
actors.Clear();
}

void INotifyOwnerChanged.OnOwnerChanged(Actor self, Player oldOwner, Player newOwner)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see this documented at all and this seems to be a very specific behavior unrelated to spawning nearby actors.
Regardless of whether this functionality remains, it should be documented IMO.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a note to the trait description.

Copy link
Member

@phrohdoh phrohdoh Dec 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that in reference to the "which keep connected?"
That makes me think this'd be used for (visually) connecting walls, not ownership of the spawned actors always being that of the actor that spawned them throughout the game.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Owner changing is still undocumented.

mods/ra2/rules/allied-structures.yaml Show resolved Hide resolved
@phrohdoh
Copy link
Member

phrohdoh commented Aug 3, 2020

SpawnNeighboringActors from a modding perspective (given the information this PR as-is makes available to modders) seems awfully similar to FreeActor, so I believe differentiating via docs/name would be beneficial.

@Mailaender
Copy link
Member Author

Updated and fixed an issue where selling the aircraft command would not remove the additional pads.

@Mailaender Mailaender requested a review from phrohdoh December 13, 2020 16:56
Copy link
Member

@phrohdoh phrohdoh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline comments regarding undocumented behavior, making the trait only as abstract/generic as necessary (this isn't engine/core where we have and expect 3rd-party reuse (and if that happens, things can be made more generic then)), how the trait logic works in general (re not seeing items added to the actors instance member), and some MiniYaml values.

OpenRA.Mods.RA2/Traits/SpawnNeighboringActors.cs Outdated Show resolved Hide resolved
actors.Clear();
}

void INotifyOwnerChanged.OnOwnerChanged(Actor self, Player oldOwner, Player newOwner)
Copy link
Member

@phrohdoh phrohdoh Dec 13, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that in reference to the "which keep connected?"
That makes me think this'd be used for (visually) connecting walls, not ownership of the spawned actors always being that of the actor that spawned them throughout the game.

mods/ra2/rules/allied-structures.yaml Show resolved Hide resolved
mods/ra2/rules/allied-structures.yaml Show resolved Hide resolved
mods/ra2/rules/allied-structures.yaml Show resolved Hide resolved
@Mailaender
Copy link
Member Author

Rebased and updated.

@MustaphaTR
Copy link
Member

Needs rebase.

@abcdefg30
Copy link
Member

Tbh I'm not sure if this PR is necessary. If this repository seems unmaintained to people, I don't think there is a reason to add this workaround, or is there? I.e. since we're not in a rush we can wait for docking to work like we need.

@Mailaender Mailaender closed this Jan 4, 2023
@Mailaender Mailaender deleted the gadock branch January 4, 2023 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Only one plane can land in the Airforce center...
7 participants